[jsweep] Clean expired_entity_cleanup_helpers.cjs#30271
Conversation
- Fix // <reference to /// <reference triple-slash directive for proper type resolution - Simplify redundant slice conditional in buildNotExpiredSection - Add comprehensive test file with 26 test cases covering all exported functions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR cleans up expired_entity_cleanup_helpers.cjs and adds a new Vitest suite for its exported helpers.
Changes:
- Fixed the
@actions/github-scripttriple-slash type reference inexpired_entity_cleanup_helpers.cjs. - Simplified the
buildNotExpiredSectiontruncation logic by always usingslice(0, 10). - Added a new
expired_entity_cleanup_helpers.test.cjssuite covering the exported helpers and constants.
Show a summary per file
| File | Description |
|---|---|
actions/setup/js/expired_entity_cleanup_helpers.cjs |
Small cleanup to type-reference metadata and not-expired summary list truncation. |
actions/setup/js/expired_entity_cleanup_helpers.test.cjs |
New Vitest coverage for delay, date validation, expiration categorization, processing, summaries, and constants. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
actions/setup/js/expired_entity_cleanup_helpers.test.cjs:145
- Because the mock always resolves to the same record and this test only checks array lengths, it would still pass if
processExpiredEntitiesaccidentally reused the first result for every entity or otherwise lost per-entity data. Returning distinct records per call would verify that each loop iteration preserves the individualprocessEntityresult.
const processEntity = vi.fn().mockResolvedValue({ status: "closed", record: { number: 1 } });
const { closed, skipped, failed } = await processExpiredEntities(entities, {
entityLabel: "Issue",
delayMs: 0,
processEntity,
});
expect(closed).toHaveLength(2);
expect(skipped).toHaveLength(0);
expect(failed).toHaveLength(0);
});
- Files reviewed: 2/2 changed files
- Comments generated: 2
| const list = notExpiredEntities.slice(0, 10); | ||
| if (notExpiredEntities.length > 10) { | ||
| section += `${notExpiredEntities.length} ${entityLabel.toLowerCase()}(s) not yet expired (showing first 10):\n\n`; |
| const start = Date.now(); | ||
| await delay(10); | ||
| expect(Date.now() - start).toBeGreaterThanOrEqual(5); |
🧪 Test Quality Sentinel ReportTest Quality Score: 61/100
Test Classification DetailsView all 26 test classifications
Flagged Tests — Suggestions
|
|
@copilot review all comments |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/942d2b30-3749-4da3-803c-fa2d7c207823 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed both review findings in commit
|
Summary
Cleans and adds comprehensive test coverage for
expired_entity_cleanup_helpers.cjs.Context type
GitHub Actions (
github-script) context — usescoreglobal injected by the actions runtime (orshim.cjs).Changes
expired_entity_cleanup_helpers.cjs/// <reference>triple-slash directive: The file had// <reference types="@actions/github-script" />(two forward slashes), which is a plain comment and does not work as a TypeScript triple-slash directive for type resolution. Fixed to/// <reference ...>(three slashes).sliceconditional:buildNotExpiredSectionhad anotExpiredEntities.length > 10 ? notExpiredEntities.slice(0, 10) : notExpiredEntitiesguard that is redundant —Array.sliceis safe when the array is shorter than the requested count. Simplified tonotExpiredEntities.slice(0, 10)directly.expired_entity_cleanup_helpers.test.cjs(new file)Added 26 test cases covering all exported functions:
delayvalidateCreationDatecategorizeByExpirationprocessExpiredEntitiesbuildExpirationSummaryValidation
npm run format:cjs✅npm run lint:cjs✅npm run typecheck✅